Add ZapLogger with option to disable stacktraces #47
Conversation
Signed-off-by: Hasan Turken <turkenh@gmail.com>
| // config will be used (stacktraces on errors, sampling). | ||
| // If disableStacktrace is true, stacktraces enabled on fatals | ||
| // independent of config. | ||
| func zapLogger(development bool, disableStacktrace bool) logr.Logger { |
There was a problem hiding this comment.
as discussed in crossplane-contrib/provider-gcp#49, I like the idea of having a common reusable way to get a logger that only does stack traces for fatal level. Can we continue pursuing that on this PR? If we finish and merge this one, then all consumer repos (main crossplane, GCP/AWS/Azure stacks) can just call this one single implementation.
pkg/logging/logging.go
Outdated
| func zapLogger(development bool, disableStacktrace bool) logr.Logger { | ||
| zl := runtimezap.RawLoggerTo(os.Stderr, development) | ||
|
|
||
| if disableStacktrace { |
There was a problem hiding this comment.
do you think this should use the new controller-runtime v0.2.2 way of initializing the logger? It is probably not worth updating the entire controller-runtime to just get the new logger initialization, but it could also be nice to get a new controller-runtime update anyways.
What is your opinion @turkenh?
There was a problem hiding this comment.
As you noted, updating controller-runtime is not necessary for this specific issue but I agree that it make sense to bump controller-runtime to v0.2.2 which is latest v0.2, currently it is on v0.2.0-beta.5 which does not sound that stable :)
Still I'll go and ask on slack dev channel to make sure everyone is ok with this.
Signed-off-by: Hasan Turken <turkenh@gmail.com>
Signed-off-by: Hasan Turken <turkenh@gmail.com>
|
I'm afraid I don't agree with @jbw976 regarding having crossplane-runtime provide helper code to produce correctly loggers. I'd rather keep any such code (which I imagine should be fairly succinct?) limited to How should we proceed with this PR? I personally don't feel anything is currently broken, so my vote would be to close it. |
Signed-off-by: Hasan Turken turkenh@gmail.com
Description of your changes
This PR is to show how stacktraces could be disabled with current revision of controller runtime. If we update controller runtime, we need to implement like in crossplane-contrib/provider-gcp#49
Related to: Related crossplane/crossplane#529
Checklist
I have:
make reviewableto ensure this PR is ready for review.clusterrole.yamlto include any new types.